Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

render-counter directive #9206

Merged
merged 4 commits into from
Nov 23, 2016
Merged

render-counter directive #9206

merged 4 commits into from
Nov 23, 2016

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Nov 23, 2016 •
edited
Loading

Implements a render-counter directive that listens for renderComplete events on it's element and tracks the number of events it sees.

This directive is attached to elements where we want to capture the render count, including the <doc-table> and <visualize> directives in discover and visualize.

If the render counter should be disabled, access it's controller and call it's disabled method like the <visualize> directive does.

@spalger spalger force-pushed the implement/doc-table-render-count branch from de75deb to 641f9af Compare November 23, 2016 19:01
@spalger
Copy link
Contributor Author

spalger commented Nov 23, 2016

jenkins, test this

@kobelb
Copy link
Contributor

kobelb commented Nov 23, 2016

LGTM

w33ble
w33ble reviewed Nov 23, 2016
View reviewed changes
src/ui/public/doc_table/doc_table.js
$el.attr('has-render-count', 'true');
$el.attr('render-count', '0');
$el.on('renderComplete', () => {
$el.attr('render-count', parseInt($el.attr('render-count'), 10) + 1);
Copy link
Contributor

@w33ble w33ble Nov 23, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a dumb micro-optimization, but reading this off the DOM every time isn't free. Why not store the value in the directive?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷

@spalger spalger changed the title [docTable] Implement render count render-counter directive Nov 23, 2016
@w33ble
Copy link
Contributor

w33ble commented Nov 23, 2016 •
edited
Loading

@spalger doesn't the doc-table directive also need the renderCounter.disable() step like visualize does? nevermind, I see what's going on here now.

@w33ble
Copy link
Contributor

w33ble commented Nov 23, 2016

LGTM!

kobelb
kobelb reviewed Nov 23, 2016
View reviewed changes
Copy link
Contributor

@kobelb kobelb left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

src/ui/public/doc_table/doc_table.js
@@ -24,7 +24,7 @@ uiModules.get('kibana')
infiniteScroll: '=?',
filter: '=?',
},
link: function ($scope) {
link: function ($scope, $el) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$el isn't needed anymore

@spalger spalger merged commit 9527a8f into elastic:master Nov 23, 2016
elastic-jasper added a commit that referenced this pull request Nov 23, 2016
@elastic-jasper
render-counter directive
f7df956 
Backports PR #9206

**Commit 1:**
[docTable/row] delay first render until watchers initialize

* Original sha: 4945e08
* Authored by spalger <[email protected]> on 2016-11-23T18:57:22Z

**Commit 2:**
[docTable/row] emit a renderComplete event, count in table directive

* Original sha: 641f9af
* Authored by spalger <[email protected]> on 2016-11-23T18:58:21Z

**Commit 3:**
[renderCounter] unify render counting login into directive

* Original sha: 0c084ac
* Authored by spalger <[email protected]> on 2016-11-23T21:26:44Z

**Commit 4:**
[docTable] remove unused var

* Original sha: 275bf70
* Authored by spalger <[email protected]> on 2016-11-23T22:00:52Z
elastic-jasper added a commit that referenced this pull request Nov 23, 2016
@elastic-jasper
render-counter directive
7e8d7c9 
Backports PR #9206

**Commit 1:**
[docTable/row] delay first render until watchers initialize

* Original sha: 4945e08
* Authored by spalger <[email protected]> on 2016-11-23T18:57:22Z

**Commit 2:**
[docTable/row] emit a renderComplete event, count in table directive

* Original sha: 641f9af
* Authored by spalger <[email protected]> on 2016-11-23T18:58:21Z

**Commit 3:**
[renderCounter] unify render counting login into directive

* Original sha: 0c084ac
* Authored by spalger <[email protected]> on 2016-11-23T21:26:44Z

**Commit 4:**
[docTable] remove unused var

* Original sha: 275bf70
* Authored by spalger <[email protected]> on 2016-11-23T22:00:52Z
This was referenced Nov 23, 2016
Merged
Merged
@spalger spalger added the v5.2.0 label Nov 23, 2016
@spalger spalger deleted the implement/doc-table-render-count branch November 23, 2016 22:09
spalger pushed a commit that referenced this pull request Nov 24, 2016
@elastic-jasper @spalger
render-counter directive (#9210)
4617e09 
Backports PR #9206

**Commit 1:**
[docTable/row] delay first render until watchers initialize

* Original sha: 4945e08
* Authored by spalger <[email protected]> on 2016-11-23T18:57:22Z

**Commit 2:**
[docTable/row] emit a renderComplete event, count in table directive

* Original sha: 641f9af
* Authored by spalger <[email protected]> on 2016-11-23T18:58:21Z

**Commit 3:**
[renderCounter] unify render counting login into directive

* Original sha: 0c084ac
* Authored by spalger <[email protected]> on 2016-11-23T21:26:44Z

**Commit 4:**
[docTable] remove unused var

* Original sha: 275bf70
* Authored by spalger <[email protected]> on 2016-11-23T22:00:52Z
spalger pushed a commit that referenced this pull request Nov 24, 2016
@elastic-jasper @spalger
render-counter directive (#9211)
5b30007 
Backports PR #9206

**Commit 1:**
[docTable/row] delay first render until watchers initialize

* Original sha: 4945e08
* Authored by spalger <[email protected]> on 2016-11-23T18:57:22Z

**Commit 2:**
[docTable/row] emit a renderComplete event, count in table directive

* Original sha: 641f9af
* Authored by spalger <[email protected]> on 2016-11-23T18:58:21Z

**Commit 3:**
[renderCounter] unify render counting login into directive

* Original sha: 0c084ac
* Authored by spalger <[email protected]> on 2016-11-23T21:26:44Z

**Commit 4:**
[docTable] remove unused var

* Original sha: 275bf70
* Authored by spalger <[email protected]> on 2016-11-23T22:00:52Z
@spalger spalger restored the implement/doc-table-render-count branch November 27, 2016 09:57
@epixa epixa added v5.1.1 and removed v5.1.1 labels Dec 8, 2016
@epixa epixa removed the v5.1.0 label Dec 8, 2016
airow pushed a commit to airow/kibana that referenced this pull request Feb 16, 2017
@elastic-jasper @spalger
render-counter directive (elastic#9210)
c118229 
Backports PR elastic#9206

**Commit 1:**
[docTable/row] delay first render until watchers initialize

* Original sha: 4945e08
* Authored by spalger <[email protected]> on 2016-11-23T18:57:22Z

**Commit 2:**
[docTable/row] emit a renderComplete event, count in table directive

* Original sha: 641f9af
* Authored by spalger <[email protected]> on 2016-11-23T18:58:21Z

**Commit 3:**
[renderCounter] unify render counting login into directive

* Original sha: 0c084ac
* Authored by spalger <[email protected]> on 2016-11-23T21:26:44Z

**Commit 4:**
[docTable] remove unused var

* Original sha: 275bf70
* Authored by spalger <[email protected]> on 2016-11-23T22:00:52Z

Former-commit-id: 4617e09
@spalger spalger deleted the implement/doc-table-render-count branch August 18, 2020 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kobelb kobelb kobelb left review comments

@w33ble w33ble w33ble left review comments

Assignees

@w33ble w33ble

@kobelb kobelb

Labels
backported review v5.1.1 v5.2.0 v6.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@spalger @kobelb @w33ble @epixa @ppisljar